feat(chart): make pod and sidecar securityContext configurable - #538

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign sgtcodfish for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Hi @yugstar. Thanks for your PR.

I'm waiting for a cert-manager member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

/ok-to-test

Hi @yugstar — thank you for the work on this. Before reviewing the chart changes in detail, I spent some time empirically testing whether the driver actually functions with privileged: false, which is what the originating issue (cert-manager/csi-driver#583) is asking for. The results are written up here: cert-manager/csi-driver#583 (comment)

The short version: every variant I could construct with privileged: false either fails Kubernetes admission, fails to start, or silently delivers an empty volume to the workload pod. The driver structurally requires mountPropagation: Bidirectional, which Kubernetes only permits on privileged containers, and a Helm values knob exposing privileged cannot change that.

I also looked for legitimate operator use cases for the other knobs in this PR (podSecurityContext, the sidecar fields, per-container runAsUser) and could not find one for this daemonset specifically:

• Pod-level fields: the main container must run as root with privileged: true, so runAsNonRoot, runAsUser, runAsGroup, fsGroup, seccompProfile and appArmorProfile are either ignored by the privileged container (seccomp/apparmor) or would break it (runAsNonRoot). The sidecars already override at container level.
• Sidecar securityContext (nodeDriverRegistrarSecurityContext, livenessProbeSecurityContext): the current hardcoded values are already maximally restrictive (allowPrivilegeEscalation: false, capabilities: drop: ALL, readOnlyRootFilesystem: true, non-root). Configurability can only weaken them.
• Main container securityContext: this is the dangerous one — the only field operators would want to change to satisfy a scanner is privileged: false, which produces the silent-failure mode in the issue write-up.

I think the right outcome is to close this PR and instead address #583 with either (1) documentation in the chart README explaining why privileged: true is structural, so operators can write scanner exemptions, or (2) an architectural change to csi-lib so the driver no longer requires Bidirectional mount propagation. The latter is non-trivial and is not a Helm change.

Happy to be shown wrong if you have an operator scenario in mind that the table in the issue comment does not cover — I would rather find a working unprivileged path than close this PR if one exists.

Edit — walking back the secondary point above

I overstated the case when I said I "could not find a legitimate operator use case" for podSecurityContext, the sidecar fields, or per-container runAsUser. After more thought there are real reasons an operator might need them, even while the main container's privileged: true requirement stays put:

• AppArmor profiles apply to privileged containers. AppArmor is an LSM independent of capabilities — the kernel enforces a profile if one is set; the operator just has to write it permissively enough for the privileged operations to work. Hardened distros like Bottlerocket and Talos ship AppArmor policies; until Kubernetes 1.30 promoted appArmorProfile to a first-class SecurityContext field there was no way to apply them via Helm, and trust-manager's #836 merged the same shape for that chart.
• SELinux labels apply to privileged containers too. Multi-tenant clusters mandating specific seLinuxOptions.level values for tenancy isolation cannot apply them via the hardcoded chart.
• The sidecars (node-driver-registrar, liveness-probe) are not privileged. Operators on platforms with a custom Localhost seccomp profile have no way to apply it today — the chart hardcodes RuntimeDefault on the sidecars. The per-sidecar fields in this PR are how they would.
• runAsUser pinning on the sidecars is a real requirement on OpenShift SCC ranges and in environments with audit-log UID alignment policies.
• fsGroup at pod level for the hostPath ownership of csi-data-dir / plugin-dir in clusters with strict ownership policies. Niche but valid.

So the narrower recommendation: keep podSecurityContext, keep the sidecar fields, keep per-container runAsUser. Drop only the main container's securityContext exposure — the silent-failure footgun documented in the issue write-up is specific to that one knob and does not generalise to the rest of the PR.

Apologies for the overshoot in the previous comment.

Thank you for taking the time to test this end to end. The writeup in #583 is really helpful and convincing. I have updated both this PR and the companion cert-manager/csi-driver#672 to match your recommendation.

What changed:

• Dropped the configurable securityContext on the main driver container. It is back to the hardcoded privileged: true, with a comment in the daemonset explaining that the container mounts pods-mount-dir with mountPropagation: Bidirectional, which Kubernetes only allows for privileged containers. That removes the privileged: false footgun you documented.
• Kept the pod-level securityContext and the two sidecar securityContext knobs (node-driver-registrar, liveness-probe). Those are the legitimate operator cases you pointed out, and they match the trust-manager #836 precedent.

The values schema, README and helm unittests were regenerated and trimmed accordingly. Happy to also document the privileged requirement in the chart README if you would prefer that over the template comment.